[AutoSparkUT] Fix GPU missing-file recovery guidance [reduced-it] - #15835
Conversation
Signed-off-by: Allen Xu <allxu@nvidia.com>
Greptile SummaryThe PR restores Spark-compatible missing-file recovery behavior in GPU file scans and preserves missing paths across asynchronous and data-source reader boundaries. The latest changes retain the concrete reader-factory identity for reader-type inspection while applying structured missing-file conversion at the execution boundary.
Confidence Score: 5/5The PR appears safe to merge; the latest factory-wrapper relocation preserves both reader-type identity and runtime missing-file conversion. No actionable new failure or repository-rule violation remains in the changes since the previous review. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[GPU batch scan] --> B[Raw partition reader factory]
B --> C[Reader-type inspection]
B --> D[MissingFileErrorShim wrapper]
D --> E[GpuDataSourceRDD]
E --> F[Columnar reader]
F -->|missing file| G[Spark-compatible recovery error]
Reviews (10): Last reviewed commit: "Preserve multi-file reader factory ident..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This pull request restores Spark-equivalent missing-file recovery guidance for GPU ORC scans when cached data files are deleted, aligning GPU error messaging with Spark’s V1/V2 expectations and re-enabling previously excluded upstream MetadataCache tests.
Changes:
- Add recovery-hint enrichment for
FileNotFoundException(includingExecutionException-wrapped cases) inGpuDataSourceRDD, with a switch to include the V1REFRESH TABLEhint. - Plumb
includeRefreshHint = truefrom V1 file-source scans (GpuFileSourceScanExec) intoGpuDataSourceRDD. - Re-enable the previously excluded Spark 3.3 MetadataCache suites and add RAPIDS-focused tests across COALESCING/MULTITHREADED ORC readers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/src/test/spark330/scala/org/apache/spark/sql/rapids/utils/RapidsTestSettings.scala | Re-enables the previously excluded MetadataCache tests now that GPU guidance is expected to match Spark. |
| tests/src/test/spark330/scala/org/apache/spark/sql/rapids/suites/RapidsMetadataCacheSuite.scala | Adds RAPIDS-specific tests asserting V1 vs V2 recovery-hint content across ORC reader modes. |
| sql-plugin/src/main/scala/org/apache/spark/sql/rapids/GpuFileSourceScanExec.scala | Passes includeRefreshHint = true for V1 file-source scans when constructing GpuDataSourceRDD. |
| sql-plugin/src/main/scala/com/nvidia/spark/rapids/shims/GpuDataSourceRDD.scala | Enriches missing-file exceptions with Spark-like recovery guidance, configurable for V1 vs V2 hint text. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| } | ||
|
|
||
| override def next(): Object = { |
Performance impact: successful next() calls retain the existing iterator work and metrics-finally path; the added handlers allocate only when a missing-file exception is thrown. Signed-off-by: Allen Xu <allxu@nvidia.com>
|
build |
Performance impact: compile-time-only API tightening; runtime policy values and exception-path behavior are unchanged. Signed-off-by: Allen Xu <allxu@nvidia.com>
|
build |
Signed-off-by: Allen Xu <allxu@nvidia.com>
Signed-off-by: Allen Xu <allxu@nvidia.com>
|
build |
1 similar comment
|
build |
Signed-off-by: Allen Xu <allxu@nvidia.com>
Resolve the GpuDataSourceRDD conflict by preserving the missing-file conversion while adopting the upstream reader lifecycle and custom metrics integration. Performance impact: the merge adds only exception-path conversion around hasNext/next and one input-partition reference update per reader; the successful row/batch path retains upstream ReaderIterator metric and close behavior. Signed-off-by: Allen Xu <allxu@nvidia.com>
Signed-off-by: Allen Xu <allxu@nvidia.com>
Signed-off-by: Allen Xu <allxu@nvidia.com>
|
build |
2 similar comments
|
build |
|
build |
Signed-off-by: Allen Xu <allxu@nvidia.com>
|
build |
JaCoCo production line coverage: independently measured, not additive across incompatible shim bytecode tuples (
sql-plugin +60, Spark 3.3/Scala 2.12;sql-plugin +48, Spark 4.0/Scala 2.13; measured head4896e367)Fixes #15511.
Description
GPU file scans can surface a raw
FileNotFoundExceptionafter a cached data file is removed. Before Spark 4, this omits Spark's recovery guidance. On Spark 4.x, it also loses structured-error parity with CPU execution: the GPU path should preserve Spark's error condition and the exact missingPATHparameter.This change handles both generations directly in the existing fix:
REFRESH TABLE/recreate guidance and the V2 recreate-only guidance.ExecutionException-wrapped missing-file failures are converted; unrelated exceptions and deeper wrapper chains are left unchanged.GpuDataSourceRDDand V2 reader factories.The three inherited Spark tests excluded by #15511 are re-enabled, and focused RAPIDS coverage exercises the relevant paths:
RapidsMetadataCacheV1Suite: SPARK-16336,SPARK-27961 Suggest fixing FileNotFoundExceptionmaps to SparkMetadataCacheSuite.scalalines 41-63.RapidsMetadataCacheV1Suite: SPARK-16337 temporary view refreshmaps to SparkMetadataCacheSuite.scalalines 72-99.RapidsMetadataCacheV2Suite: SPARK-16336,SPARK-27961 Suggest fixing FileNotFoundExceptionmaps to SparkMetadataCacheSuite.scalalines 41-63.missing ORC file includes recovery guidance - COALESCING/MULTITHREADEDverifies the V1/V2-specific guidance.MissingFileStructuredErrorSuitecovers V1 and V2 withCOALESCINGandMULTITHREADED, plus V1PERFILEand the copiedGpuFileScanRDD, comparing the CPU/GPU error condition and missing-path parameter and verifying GPU scan-plan evidence.FileSystemBytesReadTrackerSuite: GPU datasource RDD enriches next() missing-file failures - direct V2/wrapped V1directly covers reader failures and the metricsfinallypath.Original Spark source: https://github.com/apache/spark/blob/f74867bddfbcdd4d08076db36851e88b15e66556/sql/core/src/test/scala/org/apache/spark/sql/MetadataCacheSuite.scala#L41-L99
Local validation:
GpuMultiFileReaderSuite:Tests: succeeded 17, failed 0, canceled 0, ignored 0, pending 0;All tests passed;BUILD SUCCESS.GpuMultiFileReaderSuiteandMissingFileStructuredErrorSuite, ANSI off/on: each run reportedTests: succeeded 8, failed 0, canceled 0, ignored 0, pending 0;BUILD SUCCESS.No Origin.context leaks in shared source; shim signature coverage consistent across peers.sql-plugincompile:BUILD SUCCESS.sql-plugincompile:BUILD SUCCESS.spark-parent_2.13:4.0.0-databricks-173is unavailable locally.rapids-4-spark-private_2.13:spark500:26.10.0-SNAPSHOTis unavailable locally.4896e367:sql-plugin +60of 162 added production lines on Spark 3.3/Scala 2.12, andsql-plugin +48of 162 on Spark 4.0/Scala 2.13. These measurements use incompatible classfiles and are intentionally reported independently rather than summed.Performance impact: a current-code local reader-wrapper microbenchmark measured a 0.695 ns/row baseline median and 0.698 ns/row wrapped median, a +0.003 ns/row (+0.45%) delta. The successful Spark 4/5 path uses direct
try/catchboundaries aroundcreateReader,createColumnarReader,next, andgetwithout the previous call-by-name helper; it adds no filesystem access, metadata reads, or GPU operations.AI assistance: The change and PR description were prepared with Codex assistance and reviewed by the author before submission.
Checklists
Documentation
Testing
(Please provide the names of the existing tests in the PR description.)
Performance